Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios Fix static lib issues for signed device builds and iTunes connect uploads #4455

Merged
merged 1 commit into from
Mar 28, 2016

Conversation

boundsj
Copy link
Contributor

@boundsj boundsj commented Mar 24, 2016

Add a flavor of static build that:

  • Puts resources in a Mapbox.bundle file
  • Puts Mapbox.bundle and Settings.bundle at the same level as Mapbox.framework in the /static output folder
  • Removes CFBundleExecutable and CFBundlePackageType from the Info.plist in Mapbox.bundle that cause errors when uploading to iTunes Connect

This also updates the binary instructions in INSTALL.md to reflect these changes.

Fixes #4118

cc @1ec5

@boundsj
Copy link
Contributor Author

boundsj commented Mar 24, 2016

@1ec5 @friedbunny I've tested the following scenarios (all with a simple Objc client app linked to the static lib created with the proposed changes):

All of this was done with Xcode 7.3. Before these changes, only first test "Build and run on simulator" passed.

  • Build and run on simulator
  • Build and run on device (with iOS 9.3) with development cert and profile
  • Archive, verify, and upload to iTunes connect with iOS 7 as the Deployment Target (also distributed with Testflight)
  • Archive, verify, and upload to iTunes connect with iOS 8 as the Deployment Target (also distributed with Testflight)
  • Archive, verify, and upload to iTunes connect with iOS 9 as the Deployment Target (also distributed with Testflight)

screen shot 2016-03-23 at 9 22 35 pm

@@ -95,8 +95,13 @@ If your application targets iOS 7.x, you’ll need to install the static framewo

1. Build from source manually per above.

1. Open the project editor and select your application target. Drag `build/ios/pkg/static/Mapbox.framework` into the “Embedded Binaries” section of the General tab. (Don’t drag it into the “Linked Frameworks and Libraries” section; Xcode will add it there automatically.) In the sheet that appears, make sure “Copy items if needed” is checked, then click Finish.

1. Copy the contents of `build/ios/pkg/static` into your project. It should happen automatically, but ensure that:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Developers have indicated that they’d like this step to be as explicit as possible, so let’s say to drag those files into the Project navigator, checking “Copy items if needed”.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will fix.

@boundsj
Copy link
Contributor Author

boundsj commented Mar 24, 2016

@1ec5 I'm not sure why the node / android builds are now failing. I've tried rerunning but no luck so far. Will ignore since I'm certain this change set is unrelated to those builds.

@@ -207,17 +222,18 @@ SEM_VERSION=$( git describe --tags --match=ios-v*.*.* --abbrev=0 | sed 's/^ios-v
SHORT_VERSION=${SEM_VERSION%-*}
if [[ ${BUNDLE_RESOURCES} ]]; then
cp -pv LICENSE.md "${OUTPUT}/static/${NAME}.framework"
cp -rv platform/ios/app/Settings.bundle "${OUTPUT}/static/${NAME}.framework"
cp -rv platform/ios/app/Settings.bundle ${STATIC_SETTINGS_DIRECTORY}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to rethink $BUNDLE_RESOURCES. I added this variable to support make ifabric, which outputs an all-in-one static framework and nothing else. But now make ipackage needs to simultaneously output a static framework, a dynamic framework, and some auxiliary resources like Settings.bundle.

Before this change, make ipackage would place the LICENSE.md and Settings.bundle directly under pkg/, so that users of the dynamic framework can also access those resources easily. Now they’re hidden inside the pkg/static/ directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood and I agree. I'll think about how to make this better. Will check with you with some ideas tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think I'm finally with you @1ec5. So we would like something like (focusing on the static side of the fence and the files moving around in this change):

  • ifabric
    • pkg/static/Mapbox.framework (holds everything)
  • ipackage* flavors
    • pkg/
      • Mapbox.bundle
      • Settings.bundle
      • static/
    • pkg/static/
      • Mapbox.framework

I'll adjust accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More like:

  • ifabric
    • pkg/static/Mapbox.framework (holds everything)
  • ipackage* flavors
    • pkg/
      • Settings.bundle
      • *.md
      • static/
      • dynamic/
    • pkg/static/
      • Mapbox.framework
      • Mapbox.bundle (resources in here)
    • pkg/dynamic/
      • Mapbox.framework (resources in here)
      • Mapbox.framework.dSYM (for -symbols builds)

@1ec5
Copy link
Contributor

1ec5 commented Mar 24, 2016

Remember to update pod-README.md, which gets copied into the build output as README.md. This is the primary place for developers to read the installation instructions.

@1ec5
Copy link
Contributor

1ec5 commented Mar 24, 2016

I'm not sure why the node / android builds are now failing.

@jfirebaugh added a bitrise.yml to master in 231e0b5 and set up Bitrise to look for that configuration file. So that change needs to be cherry-picked into this branch; otherwise, the Node build is guaranteed to fail. Alternatively, we might be able to change the workflow script’s trigger_map.pattern to be master instead of * for now, to prevent that build from running on any other branches. A final option would be to define the workflow entirely inside Bitrise’s UI instead of in a bitrise.yml file.

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS build labels Mar 24, 2016
@1ec5 1ec5 added this to the ios-v3.2.0 milestone Mar 24, 2016
@boundsj
Copy link
Contributor Author

boundsj commented Mar 24, 2016

@1ec5 I'll make a note in the INSTALL doc that libc++.tbd, libsqlite3.tbd, libz.tbd are actually dylib files in older xcode versions

@boundsj
Copy link
Contributor Author

boundsj commented Mar 24, 2016

Passed this test case with Xcode 6.4 and a device with iOS 8.4.1

  • Archive, verify, and upload to iTunes connect with iOS 7 as the Deployment Target (also distributed with Testflight)

@boundsj
Copy link
Contributor Author

boundsj commented Mar 25, 2016

@1ec5 I took another stab at this. Please let me know if you think this will work or if a slightly larger refactor of package.sh is required. I have tested that ipackage* and ifrabric produce results as outlined above.

1. Drag the Mapbox.bundle and Mapbox.framework files in `build/ios/pkg/static` into the Project navigator, checking "Copy items if needed". It should happen automatically, but ensure that:

- `Mapbox.framework` is listed in your `Link Binary With Libraries` build phase.
- The path to the project's local copy of `Mapbox.framework` is in your *Framework Search Paths* (`FRAMEWORK_SEARCH_PATHS`) build setting.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this accurate? FRAMEWORK_SEARCH_PATHS is automatically set to $(inherited) $(PROJECT_DIR) in my test project and it seems to be working just fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the wording is just imprecise. How about:

Your *Framework Search Paths* (`FRAMEWORK_SEARCH_PATHS`) build setting includes the directory that contains `Mapbox.framework`. For most projects, the default value of `$(inherited) $(PROJECT_DIR)` should be sufficient.

@1ec5
Copy link
Contributor

1ec5 commented Mar 25, 2016

Can you add a blurb about these fixes to the changelog as well?

@boundsj
Copy link
Contributor Author

boundsj commented Mar 25, 2016

@1ec5 I think all issues are addressed. If there is nothing else, I'll merge after a +1. Thanks again!

@@ -84,6 +84,7 @@ Known issues:
- The Improve This Map tool now uses the same zoom level that is currently being shown in the map view. ([#4068](https://github.com/mapbox/mapbox-gl-native/pull/4068))
- Fixed a formatting issue in the documentation for `MGLCoordinateBoundsIsEmpty()`. ([#3958](https://github.com/mapbox/mapbox-gl-native/pull/3958))
- The maximum maximum zoom level is now 21. ([#4417](https://github.com/mapbox/mapbox-gl-native/pull/4417)))
- Fixed issues with configuration and documentation that caused problems when installing the SDK as a static binary ([#4455](https://github.com/mapbox/mapbox-gl-native/pull/4455))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s point out that this should also fix App Store submission problems, since the installation itself appears to work without a hitch until you go to release the application.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Also rebased and added this blurb to the ios specific changelog.

@1ec5
Copy link
Contributor

1ec5 commented Mar 26, 2016

👍

@1ec5
Copy link
Contributor

1ec5 commented Mar 28, 2016

This will also fix #3656.

Fix issues with static build configuration that caused it to be
problematic for host applications when they were installed on device.
Also fixes issues that broke the iTunes Connect validation and upload
process. This also updates the `binary` instructions in INSTALL.md to reflect
these changes.
@boundsj boundsj merged commit 62f636f into release-ios-3.2.0-android-4.0.0 Mar 28, 2016
@boundsj boundsj deleted the boundsj-fix-static-lib branch March 28, 2016 17:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants